-
Notifications
You must be signed in to change notification settings - Fork 22
test(e2e): Update e2e tests to support newer spec submodule #604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(e2e): Update e2e tests to support newer spec submodule #604
Conversation
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Some of the test cases have been skipped due to the SDK not having support for it yet. Some tests will fail due to a bug in the InMemoryProvider. Signed-off-by: Kyle Julian <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #604 +/- ##
=======================================
Coverage 89.89% 89.89%
=======================================
Files 77 77
Lines 3168 3168
Branches 364 364
=======================================
Hits 2848 2848
Misses 251 251
Partials 69 69 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request makes significant progress in expanding the E2E test suite to cover new scenarios from the specification's gherkin files. The introduction of test-flags.json is a good step towards data-driven testing. However, a major concern is that the test data from this JSON file is manually duplicated in C# code within BaseStepDefinitions.cs, which undermines the benefit of the JSON file and creates a maintenance challenge. Additionally, there are several opportunities to refactor duplicated code in the new step definition files, which would improve code quality and maintainability. My review includes specific suggestions to address these points.
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
aepfli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy to see, that we are moving further into the gherkin spec direction. i left some nits, i think this move to the json file, and the new format for test steps, helps us tremendiously to ensure compliance between sdks. thank you for this contribution
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
Signed-off-by: Kyle Julian <[email protected]>
This PR
Add code to handle the new scenarios covered in the specification gherkin files. Some of the scenarios are skipped as the core SDK does not have support for it yet, for example like disabled InMemoryProvider flags.
Related Issues
Notes
Follow-up Tasks
How to test